feat: Create Exercise card #1998
Conversation
|
@flacial is attempting to deploy a commit to the c0d3-prod Team on Vercel. A member of the Team first needs to authorize it. |
Codecov Report
@@ Coverage Diff @@
## master #1998 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 169 170 +1
Lines 2903 2937 +34
Branches 779 793 +14
=========================================
+ Hits 2903 2937 +34
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| } | ||
|
|
||
| const Footer = ({ exercise, onRemove }: FooterProps) => { | ||
| const [deleteExercise, { loading }] = useMutation(DELETE_EXERCISE) |
There was a problem hiding this comment.
There should be a useDeleteExerciseMutation hook you can use from graphql/index file
There was a problem hiding this comment.
I thought so too, but it doesn't seem to exist. I'm not certain of the requirements for a resolver to be auto-generated as a hook. The exercise resolvers look exactly as module resolvers
There was a problem hiding this comment.
From my understanding, its not that codegen gql generates resolvers as a hook, but rather codegen generates us hooks to use in place of useMutation. For these hooks, the functionality should be the same as useMutation, except you don't have to input the query yourself. This also means that if the query contains multiple mutation hitting different resolvers, then all those resolvers will be run.
Example of graphql/index code that inputs query for us(useGetApp):
https://github.com/garageScript/c0d3-app/blob/master/graphql/index.tsx#L2851
I think the problem might be that you haven't made codegen generate a useDeleteExerciseMutation hook yet. Please look into trying to use a useDeleteExerciseMutation hook that codegen gql generates for us.
There was a problem hiding this comment.
I think the problem might be that you haven't made codegen generate a useDeleteExerciseMutation hook yet.
So, it seems GraphQL code-gen needs a manually created document such as deleteModule to generate the hook. I'll update it in a different PR. #2006
graphql/queries/updateExercise.ts
Outdated
| $answer: String! | ||
| $moduleId: Int! | ||
| $description: String! | ||
| $flaggedAt: String |
There was a problem hiding this comment.
General practice: timestamps should be set on the server side, should not be passed in
| `; | ||
|
|
||
| ======= | ||
| >>>>>>> upstream/master |
There was a problem hiding this comment.
this looks like an unresolved merge conflict
| <> | ||
| <button | ||
| className={styles['card__section--explanation']} | ||
| onClick={() => setShow(!show)} |
There was a problem hiding this comment.
When updating state based on previous value of the state it would be better pass a function the setState to get access to the latest value of the state and prevent hard to find bugs because of race conditions, batching, etc. https://beta.reactjs.org/apis/usestate#updating-state-based-on-the-previous-state
setShow(prevShow => !prevShow) something like this for example.
| <span | ||
| className={`${styles.card__header__badge} ${styles.card__header__badge__module}`} | ||
| > | ||
| {toUpper(exercise.module.name)} |
There was a problem hiding this comment.
nit: since this is purely for appearance we shoul consider doing such modifications using the css text-transform property instead of javascript.
| const btn = screen.getByText('Show explanation') | ||
| await userEvent.click(btn) | ||
|
|
||
| expect(screen.getByText('Hide explanation')).toBeTruthy() |
There was a problem hiding this comment.
nit: The following alternatives make the intention of the test more clear and can be considered in the future of the more ambiguous toBeTruthy
expect(btn).toHaveTextContent('Hide Explanation') // Btn's text content has changed
expect(screen.getByText('Hide explanation')).toBeInTheDocument() // An element with the text 'Hide explanation' is in the document
| </MockedProvider> | ||
| ) | ||
|
|
||
| expect(screen.queryByText('Show explanation')).toBeFalsy() |
There was a problem hiding this comment.
nit: it might to be clearer to test that element does not exist
expect(screen.getByText('Show explanation')).not.toBeInTheDocument()There was a problem hiding this comment.
It makes sense. I applied it and the rest of the suggestions!
Related to #1975
Depends on #2001
Merging this PR will create the Exercise card component that will have the following functionalities:
Image: